Resolve: refactor `define` into `define_local` and `define_extern` #143884

LorrensP-2158466 · 2025-07-13T11:08:38Z

Follow up on #143550 and part of #gsoc > Project: Parallel Macro Expansion.

Split up define into define_local and define_extern. Refactor usages of define into either one where it's "correct" (idk if everything is correct atm). Big part of this is that resolution can now take a &Resolver instead of a mutable one.

Needed the module changes of #143550. Review will probably be easier when viewing my first commit instead of all the changes.

r? @petrochenkov

compiler/rustc_resolve/src/build_reduced_graph.rs

LorrensP-2158466 · 2025-07-13T11:10:28Z

compiler/rustc_resolve/src/imports.rs

@@ -477,7 +492,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
            if self.is_accessible_from(binding.vis, scope) {
                let imported_binding = self.import(binding, *import);
                let key = BindingKey { ident, ..key };
-                let _ = self.try_define(
+                // FIXME: local or external?
+                let _ = self.try_define_local(


This is in update_resolutions, I don't know if this is an equivalent change.

You could add asserts to (try_)define_(local,extern) making sure that the passed parent, res and vis are actually local or external, respectively.

I don't know how I should check if vis is local or not.

It it's Public it's ok, if it's Restricted(DefId), then it's local if the def-id is local.

Currently define_local accepts Visibility so that should be fine. But _extern still needs the check, thanks!

LorrensP-2158466 · 2025-07-13T11:11:21Z

compiler/rustc_resolve/src/lib.rs

@@ -1078,7 +1078,7 @@ pub struct Resolver<'ra, 'tcx> {
    extern_module_map: RefCell<FxIndexMap<DefId, Module<'ra>>>,
    binding_parent_modules: FxHashMap<NameBinding<'ra>, Module<'ra>>,

-    underscore_disambiguator: u32,
+    underscore_disambiguator: Cell<u32>,


This is correct, right?

No.
The local disambiguator shouldn't be used for external underscore names.
Such names should either never appear in external module children (but it seems like we cannot do that, unfortunately), or we should assign disambiguators to them before writing them to metadata.

Apparently the current resolver logic relies on all disambiguators being different for all local and loaded external names.
I'll think what to do with this.

Addressed in #144013.

compiler/rustc_resolve/src/imports.rs

bors · 2025-07-13T19:39:17Z

☔ The latest upstream changes (presumably #143888) made this pull request unmergeable. Please resolve the merge conflicts.

LorrensP-2158466 · 2025-07-13T20:35:22Z

Conflicts must be resolved in Petrochenkov's PRs, I think. I can then rebase off that.

petrochenkov · 2025-07-14T13:21:34Z

Blocked on #143550.
@rustbot blocked

petrochenkov · 2025-07-14T15:26:56Z

compiler/rustc_resolve/src/build_reduced_graph.rs

+    ) {
+        let binding = self.arenas.new_res_binding(res, vis.to_def_id(), span, expn_id);
+        let key = self.new_disambiguated_key(ident, ns);
+        self.check_reserved_macro_name(key.ident, binding.res());


This is not needed for external items.

resolve: Use interior mutability for extern module map Module map for extern modules is a lazily populated cache, it's not *significantly* mutable. If some logic in name resolver is parallelized, then this cache can be populated from any thread, and without affecting results of any speculative resolution. Unblocks rust-lang#143884. This is a part of [#gsoc > Project: Parallel Macro Expansion](https://rust-lang.zulipchat.com/#narrow/channel/421156-gsoc/topic/Project.3A.20Parallel.20Macro.20Expansion/with/527348747). cc `@LorrensP-2158466`

Rollup merge of #143550 - petrochenkov:lessmutres, r=lcnr resolve: Use interior mutability for extern module map Module map for extern modules is a lazily populated cache, it's not *significantly* mutable. If some logic in name resolver is parallelized, then this cache can be populated from any thread, and without affecting results of any speculative resolution. Unblocks #143884. This is a part of [#gsoc > Project: Parallel Macro Expansion](https://rust-lang.zulipchat.com/#narrow/channel/421156-gsoc/topic/Project.3A.20Parallel.20Macro.20Expansion/with/527348747). cc `@LorrensP-2158466`

LorrensP-2158466 · 2025-07-17T18:49:56Z

I couldn't find any more &mut Resolver that I could turn into &Resolver. Everything now either has finalize codepaths and/or lints/errors getting added to their respective collections.

resolve: Make disambiguators for underscore bindings module-local Disambiguators attached to underscore name bindings (like `const _: u8 = something;`) do not need to be globally unique, they just need to be unique inside the module in which they live, because the bindings in a module are basically kept as `Map<BindingKey, SomeData>`. Also, the specific values of the disambiguators are not important, so a glob import of `const _` may have a different disambiguator than the original `const _` itself. So in this PR the disambiguator is just set to the current number of bindings in the module. This removes one more piece of global mutable state from resolver and unblocks rust-lang#143884.

resolve: Use interior mutability for extern module map Module map for extern modules is a lazily populated cache, it's not *significantly* mutable. If some logic in name resolver is parallelized, then this cache can be populated from any thread, and without affecting results of any speculative resolution. Unblocks rust-lang/rust#143884. This is a part of [#gsoc > Project: Parallel Macro Expansion](https://rust-lang.zulipchat.com/#narrow/channel/421156-gsoc/topic/Project.3A.20Parallel.20Macro.20Expansion/with/527348747). cc `@LorrensP-2158466`

rustbot assigned petrochenkov Jul 13, 2025

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 13, 2025

LorrensP-2158466 commented Jul 13, 2025

View reviewed changes

compiler/rustc_resolve/src/build_reduced_graph.rs Outdated Show resolved Hide resolved

LorrensP-2158466 commented Jul 13, 2025

View reviewed changes

compiler/rustc_resolve/src/imports.rs Outdated Show resolved Hide resolved

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 14, 2025

petrochenkov reviewed Jul 14, 2025

View reviewed changes

This was referenced Jul 16, 2025

resolve: Make disambiguators for underscore bindings module-local #144013

Open

resolve: Use interior mutability for extern module map #143550

Merged

LorrensP-2158466 force-pushed the resolve-split-define branch from 8515b3c to 1296920 Compare July 17, 2025 15:57

This comment has been minimized.

Sign in to view

LorrensP-2158466 and others added 7 commits July 17, 2025 20:46

split up define into define_extern and define_local

752fd20

cleanup old stuff

dd8b1d1

address review

31319cb

check that vis is extern aswell

4a9cbb1

split for_each_child into immutable and mutable versions

253410f

resolve: Make disambiguators for underscore bindings module-local

9ac9526

some cleanup/rename

4497a13

LorrensP-2158466 force-pushed the resolve-split-define branch from 89cf98b to 4497a13 Compare July 17, 2025 18:47

LorrensP-2158466 marked this pull request as ready for review July 17, 2025 19:57

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 17, 2025

LorrensP-2158466 changed the title ~~[WIP] Resolve: refactor define into define_local and define_extern~~ Resolve: refactor define into define_local and define_extern Jul 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Resolve: refactor `define` into `define_local` and `define_extern` #143884

Resolve: refactor `define` into `define_local` and `define_extern` #143884

LorrensP-2158466 commented Jul 13, 2025 •

edited by rustbot

Loading

Uh oh!

Uh oh!

LorrensP-2158466 Jul 13, 2025

Uh oh!

petrochenkov Jul 14, 2025

Uh oh!

LorrensP-2158466 Jul 14, 2025

Uh oh!

petrochenkov Jul 15, 2025

Uh oh!

LorrensP-2158466 Jul 15, 2025

Uh oh!

LorrensP-2158466 Jul 13, 2025

Uh oh!

petrochenkov Jul 14, 2025 •

edited

Loading

Uh oh!

petrochenkov Jul 14, 2025

Uh oh!

petrochenkov Jul 16, 2025

Uh oh!

Uh oh!

bors commented Jul 13, 2025

Uh oh!

LorrensP-2158466 commented Jul 13, 2025 •

edited

Loading

Uh oh!

petrochenkov commented Jul 14, 2025

Uh oh!

petrochenkov Jul 14, 2025

Uh oh!

This comment has been minimized.

LorrensP-2158466 commented Jul 17, 2025

Uh oh!

Uh oh!

Resolve: refactor define into define_local and define_extern #143884

Are you sure you want to change the base?

Resolve: refactor define into define_local and define_extern #143884

Conversation

LorrensP-2158466 commented Jul 13, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petrochenkov Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bors commented Jul 13, 2025

Uh oh!

LorrensP-2158466 commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Jul 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

LorrensP-2158466 commented Jul 17, 2025

Uh oh!

Uh oh!

Resolve: refactor `define` into `define_local` and `define_extern` #143884

Resolve: refactor `define` into `define_local` and `define_extern` #143884

LorrensP-2158466 commented Jul 13, 2025 •

edited by rustbot

Loading

petrochenkov Jul 14, 2025 •

edited

Loading

LorrensP-2158466 commented Jul 13, 2025 •

edited

Loading